Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

irmin: add a Repo.close operation to the high-level API #845

Merged
merged 1 commit into from
Aug 30, 2019

Conversation

samoht
Copy link
Member

@samoht samoht commented Aug 30, 2019

This needs backends to provide their own close implementation.

Currently it's a no-op for most backends but would be good to add
consistent exceptions when users try to deal with an empty store.
This is demonstrated in the irmin-mem backend where every operation
is not checked against a closed flag stored in the repository state.

@craigfe
Copy link
Member

craigfe commented Aug 30, 2019

I wonder if (in future) we could prevent backend developers from needing to check the closed flag on each operation by pushing this logic into a Make functor somewhere? It seems to me that the only logic that will have to change from backend to backend is a free: unit -> unit Lwt.t hook.

This PR is obviously an improvement on the current state, regardless.

@@ -54,6 +54,54 @@ module Path = struct
module type S = S.PATH
end

exception Closed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this exception any more

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be raised by the Make_ext functor now! (not by the backends)

@samoht
Copy link
Member Author

samoht commented Aug 30, 2019

As suggested, I've force-pushed a solution which raise Closed in the Make_ext functor, for all the kind of store backends.

I've also removed the exception raised on multiple close, so that double-closing will just do nothing.

Copy link
Member

@craigfe craigfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


let close t =
if !(t.closed) then Lwt.return_unit
else (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, I think the OCamlformat parentheses here are quite ugly

@samoht samoht mentioned this pull request Aug 30, 2019
@samoht samoht merged commit 9080ab8 into mirage:master Aug 30, 2019
@samoht samoht deleted the close branch August 30, 2019 13:19
craigfe added a commit to craigfe/irmin.io that referenced this pull request Sep 4, 2019
Specifically, backends must now provide a `close` function:
mirage/irmin#845
craigfe added a commit to craigfe/irmin.io that referenced this pull request Sep 4, 2019
Specifically, backends must now provide a `close` function:
mirage/irmin#845
craigfe added a commit to craigfe/irmin.io that referenced this pull request Sep 4, 2019
Specifically, backends must now provide a `close` function:
mirage/irmin#845
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 20, 2019
…t, irmin-chunk, irmin-pack, irmin-mirage-graphql, irmin-mirage, irmin-graphql, irmin-http, irmin-git and irmin-test (2.0.0)

CHANGES:

#### Added

- **irmin-pack** (_new_):
  - Created a new Irmin backend, `irmin-pack`, which uses a space-optimised
    on-disk format.

- **irmin-graphql** (_new_):
  - Created a new package, `irmin-graphql`, which provides a GraphQL server
    implementation that can be used with both the MirageOS and Unix backends.
    Additionally, a `graphql` command has been added to the command-line
    interface for starting `irmin-graphql` servers. (mirage/irmin#558, @andreas, @zshipko)

  - Contents can now be queried directly using `irmin-graphql` with
    `Irmin_graphql.Server.Make_ext` and the `Irmin_graphql.Server.PRESENTER`
    interface. (mirage/irmin#643, @andreas)

- **irmin-test** (_new_):
  - Added a new package, `irmin-test`, which allows for packages to access the
    Irmin test-suite. This package can now be used for new packages that
    implement custom backends to test their implementations against the same
    tests that the core backends are tested against. (mirage/irmin#508, @zshipko)

- **irmin-unix**:
  - Add `Cli` module to expose some methods to simplify building command-line
    interfaces using Irmin. (mirage/irmin#517, @zshipko)

  - Add global config file `$HOME/.irmin/config.yml` which may be overridden by
    either `$PWD/.irmin.yml` or by passing `--config <PATH>`. See `irmin help
    irmin.yml` for details. (mirage/irmin#513, @zshipko)

- **irmin-git**:
  - Allow import/export of Git repositories using Irmin slices. (mirage/irmin#561, @samoht)

- **irmin-http**:
  - Expose a `/trees/merge` route for server-side merge operations. (mirage/irmin#714,
    @samoht)

- **irmin**:
  - Add `Json_value` and `Json` content types. (mirage/irmin#516 mirage/irmin#694, @zshipko)

  - Add optional seed parameter to the `Irmin.Type` generic hash functions.
    (mirage/irmin#712, @samoht)

  - Add `V1` submodules in `Commit`, `Contents` and `Hash` to provide
    compatibility with 1.x serialisation formats. (mirage/irmin#644 mirage/irmin#666, @samoht)

  - Add `Store.last_modified` function, which provides a list of commits where
    the given key was modified last. (mirage/irmin#617, @pascutto)

  - Add a `Content_addressable.unsafe_add` function allowing the key of the new
    value to be specified explicitly (for performance reasons). (mirage/irmin#783, @samoht)

  - Add `save_contents` function for saving contents to the database. (mirage/irmin#689,
    @samoht)

  - Add pretty-printers for the results of Sync operations. (mirage/irmin#789, @craigfe)

  - `Private.Lock` now exposes a `stats` function returning the number of held
    locks. (mirage/irmin#704, @samoht)

#### Changed

- **irmin-unix**:
  - Rename `irmin read` to `irmin get` and `irmin write` to `irmin set`. (mirage/irmin#501,
  @zshipko)

  - Switch from custom configuration format to YAML. (mirage/irmin#504, @zshipko)

- **irmin-git**:
  - Require `ocaml-git >= 2.0`. (mirage/irmin#545, @samoht)

  - Cleanup handling of remote stores. (mirage/irmin#552, @samoht)

- **irmin-http**:
  - Rename `CLIENT` to `HTTP_CLIENT` and simplify the signatures necessary to
    construct HTTP clients and servers. (mirage/irmin#701, @samoht)

- **irmin-mirage**
  - Split `irmin-mirage` into `irmin-{mirage,mirage-git,mirage-graphql}` to
    allow for more granular dependency selection. Any instances of
    `Irmin_mirage.Git` should be replaced with `Irmin_mirage_git`. (mirage/irmin#686,
    @zshipko)

- **irmin**:
  - Update to use dune (mirage/irmin#534, @samoht) and opam 2.0. (mirage/irmin#583, @samoht)

  - Replace `Irmin.Contents.S0` with `Irmin.Type.S`.

  - Rename `Type.pre_digest` -> `Type.pre_hash` and `Type.hash` ->
    `Type.short_hash`. (mirage/irmin#720, @samoht)

  - Change `Irmin.Type` to use _incremental_ hash functions (functions of type
    `'a -> (string -> unit) -> unit`) for performance reasons. (mirage/irmin#751, @samoht)

  - Simplify the `Irmin.Type.like` constructor and add a new `Irmin.Type.map`
    with the previous behaviour.

  - Improvements to `Irmin.Type` combinators. (mirage/irmin#550 mirage/irmin#538 mirage/irmin#652 mirage/irmin#653 mirage/irmin#655 mirage/irmin#656
    mirage/irmin#688, @samoht)

  - Modify `Store.set` to return a result type and create a new `Store.set_exn`
    with the previous exception-raising behaviour. (mirage/irmin#572, @samoht)

  - Rename store module types to be more descriptive:
     - replace `Irmin.AO` with `Irmin.CONTENT_ADDRESSABLE_STORE`;
     - replace `Irmin.AO_MAKER` with `Irmin.CONTENT_ADDRESSABLE_STORE_MAKER`;
     - replace `Irmin.RW` with `Irmin.ATOMIC_WRITE_STORE`;
     - replace `Irmin.RW_MAKER` with `Irmin.ATOMIC_WRITE_STORE_MAKER`. (mirage/irmin#601,
       @samoht)

  - Rename `export_tree` to `save_tree` (mirage/irmin#689, @samoht) and add an option to
    conditionally clear the tree cache (mirage/irmin#702 mirage/irmin#725, @samoht).

  - Change hash function for `Irmin_{fs,mem,unix}.KV` to BLAKE2b rather than
    SHA1 for security reasons. (mirage/irmin#811, @craigfe)

  - Move `Irmin.remote_uri` to `Store.remote`, for stores that support remote
    operations. (mirage/irmin#552, @samoht)

  - Simplify the error cases of fetch/pull/push operations. (mirage/irmin#684, @zshipko)

  - A `batch` function has been added to the backend definition to allow for
    better control over how groups of operations are processed. (mirage/irmin#609, @samoht)

  - A `close` function has been added to allow backends to close any held
    resources (e.g. file descriptors for the `FS` backend). (mirage/irmin#845, @samoht)

  - Simplify `Private.Node.Make` parameters to use a simpler notion of 'path' in
    terms of a list of steps. (mirage/irmin#645, @samoht)

  - Rename `Node.update` to `Node.add`. (mirage/irmin#713, @samoht)

#### Fixed

- **irmin-unix**:
   - Fix parsing of commit hashes in `revert` command. (mirage/irmin#496, @zshipko)

- **irmin-git**:
  - Fix `Node.add` to preserve sharing. (mirage/irmin#802, @samoht)

- **irmin-http**:
  - Respond with a 404 if a non-existent resource is requested. (mirage/irmin#706, @samoht)

- **irmin**:
  - Fix a bug whereby `S.History.is_empty` would return `true` for a store with
    exactly one commit. (mirage/irmin#865, @pascutto)

#### Removed

- **irmin**:
  - Remove `pp` and `of_string` functions from `Irmin.Contents.S` in favour of
    `Irmin.Type.to_string` and `Irmin.Type.of_string`.

  - Remove `Bytes` content type. (mirage/irmin#708, @samoht)

  - Remove `Cstruct` dependency and content type. If possible, switch to
    `Irmin.Contents.String` or else use `Irmin.Type.map` to wrap the Cstruct
    type. (mirage/irmin#544, @samoht)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants